Codecov Report

❗ No coverage uploaded for pull request base (master@272ff84). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master    #4844   +/-   ##
=========================================
  Coverage          ?   59.34%           
=========================================
  Files             ?      201           
  Lines             ?     6698           
  Branches          ?        4           
=========================================
  Hits              ?     3975           
  Misses            ?     2723           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 272ff84...20f40dd. Read the comment docs.

I'm not sure if this is the correct fix for the problem. I think this issue is more generally solved using something like https://www.npmjs.com/package/babel-jest-assertions? It will only fail when running the test instead of telling you inline in the IDE. But it will cover more cases (like if the catch is not actually called as the promised resolved)

A lint rule might be to always expect a expect.assertions(num).

@SimenB thanks for letting me know about that super-cool plugin 👍
It looks to be more in general way to solve this problem. I did give it a try and surprisingly, this was not working in this case, below is what happened,

1. There was a misunderstanding on my side that expect statement does not gets executed when you don't return the promise to be tested, that seems to be incorrect,
   example:

    it('should say hi on getting data', () => {
      getData().then((data) => {
      	console.log("The expect will be executed")
        expect(data).toEqual("Hi");
      });
    });

Here I was able to see "The expect will be executed" getting printed on console. And there is no effect on jest reporting, if I change the expectation of 'data' to 'any string' instead of 'Hi'

2. If i change the above example and add a return statement, then,

    it('should say hi on getting data', () => {
      return getData().then((data) => {
      	console.log("The expect will be executed")
        expect(data).toEqual("Hi");
      });
    });

Here I was able to see "The expect will be executed" getting printed on console. And there was effect on jest reporting, if I change the expectation of 'data' to 'any string' instead of 'Hi'

3. I tried an example, similar to one mentioned on that plugins read me page, like below,
   Note: Here I am directly using expect.hasAssertions(), just for simplicity in understanding,

it('works with resolves', () => {
  expect.hasAssertions();
  Promise.reject(10).then(value => {
    expect(value).toBe(1)
  })
});

Here, as the promise is rejected, expect.hasAssertions() will fail the test, since no assertion is executed.
if we resolve the promise as below,

it('works with resolves', () => {
  expect.hasAssertions();
  Promise.resolve(10).then(value => {
    expect(value).tobe(1)
  })
});

Here, ideally test should fail as value is '10'. But as said earlier this assertion gets executed but jest honors that only when we return that Promise. If I add return to above example test will fail for value not be 1

In the end, this lint rule looks to be required even if you use that plugin, as I discovered today that, assertion may or may not get executed but jest will honor result on that assertion only if return statement is added.

@SimenB @cpojer @thymikee please let me know if my understanding is correct.

As per above analysis, combination of this lint rule + this plugin would make it more seamless,

1. Plugin will make sure that assertion statement is always executed
2. Lint rule, will make sure that assertion statement result is honored by jest

@tushardhole I could be wrong here but I think the issue you are seeing is because you aren't marking the callback (test function) as asynchronous with either async/await or providing a done argument.

describe('async', () => {
  const getData = () => Promise.resolve('Hello');

  it('should fail as hello is returned when getting data (using done callback)', (done) => {
    getData().then((data) => {
      console.log("The expect will be executed")
      expect(data).toEqual("Hi");
      done();
    });
  });

  it('should fail as hello is returned when getting data (using async/await)', async () => {
    await getData().then((data) => {
      console.log("The expect will be executed")
      expect(data).toEqual("Hi");
    });
  });
});

@mattphillips that example works.

To conclude this as per my understanding, the plugin will work for this this issue, if the test is written using either async/await or providing a done.

In case a developer do not do this, which is possible as human error, this lint rule would be still helpful.

I do not want to be biased to get this PR merged, I feel still it makes sense to to have this lint rule and I would be interested to hear feedback from the community. 😄

If we all feel this lint rule is not required and the plugin with async/await is the way to go ahead, then no issues to close the PR.

We've decided to move the linting package out of this repo, and into https://github.com/jest-community/eslint-plugin-jest. Could you please open this PR against that repo instead? Thanks!

For people following along: jest-community/eslint-plugin-jest#42

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.